Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make multiple settings dynamic for tuning on larger clusters #16347

Merged
merged 10 commits into from
Oct 23, 2024
Merged

Make multiple settings dynamic for tuning on larger clusters #16347

merged 10 commits into from
Oct 23, 2024

Conversation

rahulkarajgikar
Copy link
Contributor

@rahulkarajgikar rahulkarajgikar commented Oct 16, 2024 •
edited
Loading

Description

Makes the below list of settings into dynamic settings:

cluster.fault_detection.follower_check.interval
cluster.allocator.gateway.batch_size
cluster.publish.timeout
cluster.follower_lag.timeout

This allows us to easily tune these settings on larger clusters, where the default settings may not be high enough

Increases max value of cluster.fault_detection.follower_check.timeout from 60s to 150s so that we can tune for larger clusters if needed

Added UTs for each of the settings to validate dynamic setting functionality

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@github-actions GitHub Actions
Copy link
Contributor

✅ Gradle check result for 3ef5783: SUCCESS

@codecov Codecov
Copy link

codecov bot commented Oct 16, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.93%. Comparing base (35c366d) to head (7d692a2).
Report is 36 commits behind head on main.

Additional details and impacted files 
@@             Coverage Diff              @@
##               main   #16347      +/-   ##
============================================
- Coverage     72.10%   71.93%   -0.18%     
+ Complexity    64862    64760     -102     
============================================
  Files          5307     5307              
  Lines        302606   302643      +37     
  Branches      43717    43720       +3     
============================================
- Hits         218208   217700     -508     
- Misses        66541    67019     +478     
- Partials      17857    17924      +67

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rahulkarajgikar rahulkarajgikar changed the title Make multiple settings dynamic for larger clusters [DRAFT] Make multiple settings dynamic for larger clusters Oct 17, 2024
Remove settings which can't be dynamic
561ac0e 
Signed-off-by: Rahul Karajgikar <[email protected]>
@github-actions GitHub Actions
Copy link
Contributor

❌ Gradle check result for 561ac0e: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Remove extra code
b332424 
Signed-off-by: Rahul Karajgikar <[email protected]>
@github-actions GitHub Actions
Copy link
Contributor

❌ Gradle check result for b332424: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Add tests for dynamic settings
c96d677 
Signed-off-by: Rahul Karajgikar <[email protected]>
@github-actions GitHub Actions
Copy link
Contributor

❌ Gradle check result for c96d677: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

spotless apply
6b6d11d 
Signed-off-by: Rahul Karajgikar <[email protected]>
@github-actions GitHub Actions
Copy link
Contributor

✅ Gradle check result for 6b6d11d: SUCCESS

Make lag detector setting dynamic
0f29002 
Signed-off-by: Rahul Karajgikar <[email protected]>
@rahulkarajgikar rahulkarajgikar changed the title [DRAFT] Make multiple settings dynamic for larger clusters [DRAFT] Make multiple settings dynamic for tuning on larger clusters Oct 17, 2024
Add changelog
c42004f 
Signed-off-by: Rahul Karajgikar <[email protected]>
@github-actions GitHub Actions
Copy link
Contributor

✅ Gradle check result for c42004f: SUCCESS

@opensearch-ci-bot opensearch-ci-bot mentioned this pull request Oct 17, 2024
Open
@rahulkarajgikar rahulkarajgikar marked this pull request as ready for review October 18, 2024 04:52
@rahulkarajgikar rahulkarajgikar changed the title [DRAFT] Make multiple settings dynamic for tuning on larger clusters Make multiple settings dynamic for tuning on larger clusters Oct 18, 2024
change protected to private
b5ca8de 
Signed-off-by: Rahul Karajgikar <[email protected]>
@github-actions GitHub Actions
Copy link
Contributor

✅ Gradle check result for b5ca8de: SUCCESS

@shwetathareja shwetathareja added the backport 2.x Backport to 2.x branch label Oct 22, 2024
@shwetathareja
cleaned up the trace log
7d692a2 
Signed-off-by: shwetathareja <[email protected]>
@shwetathareja
Copy link
Member

this will need documentation update PR @rahulkarajgikar

@github-actions GitHub Actions
Copy link
Contributor

❌ Gradle check result for 7d692a2: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@shwetathareja shwetathareja mentioned this pull request Oct 22, 2024
Closed
@shwetathareja shwetathareja added the v2.18.0 Issues and PRs related to version 2.18.0 label Oct 22, 2024
@github-actions GitHub Actions
Copy link
Contributor

❕ Gradle check result for 7d692a2: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

This was referenced Oct 22, 2024
Merged
Closed
@rahulkarajgikar
Copy link
Contributor Author

added documentation PR: opensearch-project/documentation-website#8604

@shwetathareja shwetathareja merged commit ca40ba4 into opensearch-project:main Oct 23, 2024
42 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 23, 2024
@github-actions
Make multiple settings dynamic for tuning on larger clusters (#16347)
961246c 
Signed-off-by: Rahul Karajgikar <[email protected]>
(cherry picked from commit ca40ba4)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Oct 23, 2024
Merged
dbwiddis pushed a commit that referenced this pull request Oct 23, 2024
@opensearch-trigger-bot @github-actions
Make multiple settings dynamic for tuning on larger clusters (#16347) (…
b1f2ff8 
…#16442)

(cherry picked from commit ca40ba4)

Signed-off-by: Rahul Karajgikar <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@BrewTestBot BrewTestBot mentioned this pull request Nov 6, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sumitasr sumitasr sumitasr approved these changes

@shwetathareja shwetathareja shwetathareja approved these changes

@anasalkouz anasalkouz Awaiting requested review from anasalkouz anasalkouz is a code owner

@andrross andrross Awaiting requested review from andrross andrross is a code owner

@ashking94 ashking94 Awaiting requested review from ashking94 ashking94 is a code owner

@Bukhtawar Bukhtawar Awaiting requested review from Bukhtawar Bukhtawar is a code owner

@CEHENKLE CEHENKLE Awaiting requested review from CEHENKLE CEHENKLE is a code owner

@dblock dblock Awaiting requested review from dblock dblock is a code owner

@dbwiddis dbwiddis Awaiting requested review from dbwiddis dbwiddis is a code owner

@gbbafna gbbafna Awaiting requested review from gbbafna gbbafna is a code owner

@jainankitk jainankitk Awaiting requested review from jainankitk jainankitk is a code owner

@kotwanikunal kotwanikunal Awaiting requested review from kotwanikunal kotwanikunal is a code owner

@linuxpi linuxpi Awaiting requested review from linuxpi linuxpi is a code owner

@mch2 mch2 Awaiting requested review from mch2 mch2 is a code owner

@msfroh msfroh Awaiting requested review from msfroh msfroh is a code owner

@nknize nknize Awaiting requested review from nknize nknize is a code owner

@owaiskazi19 owaiskazi19 Awaiting requested review from owaiskazi19 owaiskazi19 is a code owner

@reta reta Awaiting requested review from reta reta is a code owner

@Rishikesh1159 Rishikesh1159 Awaiting requested review from Rishikesh1159 Rishikesh1159 is a code owner

@sachinpkale sachinpkale Awaiting requested review from sachinpkale sachinpkale is a code owner

@saratvemulapalli saratvemulapalli Awaiting requested review from saratvemulapalli saratvemulapalli is a code owner

@sohami sohami Awaiting requested review from sohami sohami is a code owner

@VachaShah VachaShah Awaiting requested review from VachaShah VachaShah is a code owner

Assignees
No one assigned
Labels
backport 2.x Backport to 2.x branch v2.18.0 Issues and PRs related to version 2.18.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rahulkarajgikar @shwetathareja @sumitasr